Skip to content

fix(xugudb): escape SQL identifiers and literals in metadata/DDL paths (#1914) - #2175

Merged
openai0229 merged 8 commits into
OtterMind:mainfrom
HandSonic:fix/sqli-xugudb
Jul 29, 2026
Merged

fix(xugudb): escape SQL identifiers and literals in metadata/DDL paths (#1914)#2175
openai0229 merged 8 commits into
OtterMind:mainfrom
HandSonic:fix/sqli-xugudb

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #1914

Summary

Completes the XuguDB database-plugin portion of the identifier and literal escaping work.

  • Adds XugudbIdentifierProcessor with conditional SPI quoting, DDL-only always quoting, locale-independent reserved-word matching, null preservation, and embedded/boundary double-quote round trips.
  • Replaces regex-based default handling with quote-aware balanced scanners that preserve nested functions, casts, sequence expressions, quoted literals, and parenthesized expressions while rejecting comments, statement boundaries, unbalanced input, top-level commas, and column-clause injection.
  • Quotes qualified table paths and update/primary-key columns across inherited builder, manager, metadata, create/alter/index, drop, truncate, copy, and template paths.
  • Preserves case-only table/column renames, validates unknown type expressions in real builder paths, normalizes index sort direction, and keeps primary-key drop behavior case independent.
  • Consolidates XuguDB string-literal serialization in one processor.

Affected surfaces

  • Frontend / Web
  • Backend / API / Storage
  • Database plugin / Driver
  • JCEF / Desktop packaging
  • CI / Build / Release
  • Documentation only

Verification

  • Commands and results:
    • mvn -B -f chat2db-community-server/pom.xml -pl chat2db-community-plugins/chat2db-community-xugudb -am clean test -DskipTests=false -Dmaven.test.skip=false -Dsurefire.failIfNoSpecifiedTests=false
    • Result: BUILD SUCCESS across 9 reactor modules; 195 tests passed in tested modules, including XuguDB 35/35, Oracle 44/44, SPI 90/90, and tools 26/26.
    • git diff --check origin/main...HEAD: passed.
  • Manual verification: Source-level generated-SQL review completed. No live XuguDB instance was used.
  • UI evidence: N/A

Risk and compatibility

  • Public API or stored data: No public API or stored-data schema change. Uses the existing shared ISQLIdentifierProcessor contract.
  • Database or driver compatibility: Conditional lookup/completion quoting is preserved; generated DDL always quotes raw metadata names. Nested legal defaults and user-defined/parameterized types are covered by regression tests.
  • Network, privacy, or security: No network or privacy change. Reduces malformed SQL and identifier/literal injection risk in metadata-driven DDL, export, and DML paths.
  • Community / Local / Pro boundary: Community XuguDB plugin only; Local and Pro consume the Community plugin behavior without product-specific code changes.
  • Backward compatibility: Ordinary identifiers and values retain their emitted form; mixed-case names, embedded delimiters, reserved words, and case-only renames now preserve object identity.

Reviewer map

  • Start here: XugudbIdentifierProcessor, XugudbSqlGuards, then XUGUDBSqlBuilder, XUGUDBMetaData, and the type/index enums.
  • Failure condition: Quote/remove loses raw text, legal nested defaults/types are rewritten, generated SQL contains unquoted metadata identifiers, or update/primary-key column names bypass quoting.
  • Rollback or disable path: Revert this PR; no migration or persisted-data rollback is required.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: Substantial AI-assisted maintainer revision, multi-agent review, and test expansion; all final changes were source-reviewed and verified with the commands above.

@HandSonic
HandSonic requested a review from openai0229 as a code owner July 26, 2026 09:57
@openai0229 openai0229 moved this to In Review in Chat2DB Community Jul 26, 2026
@HandSonic
HandSonic marked this pull request as draft July 26, 2026 10:30
@openai0229 openai0229 moved this from In Review to In Progress in Chat2DB Community Jul 26, 2026
@HandSonic
HandSonic marked this pull request as ready for review July 26, 2026 11:05
@openai0229 openai0229 moved this from In Progress to In Review in Chat2DB Community Jul 26, 2026
HandSonic and others added 8 commits July 29, 2026 17:08
…olumn fallback (OtterMind#1914)

- Override quoteIdentifier/quoteQualifiedIdentifier/buildSelectTable/buildTableName/
  buildColumns in XUGUDBSqlBuilder so inherited DefaultSqlBuilder DQL/DML paths
  (exportTableData, insert, update) no longer concatenate raw identifiers
- Replace raw IColumnBuilder.buildDefaultColumn fallback with escaped name +
  anchored type-shape validation (reject otherwise)
- Return trimmed values from default-value/unit validators
- Document raw-identifier contract on XugudbSqlEscapes.escapeIdentifier
- Add attack-string tests: malicious schema/table/column via buildSelectTable and
  buildInsert, comment literal end-to-end, fallback name/type, validator trimming
… tests (OtterMind#1914)

- DEFAULT_VALUE_PATTERN function-call branch now requires balanced
  single-quoted literals as arguments ('([^']|'')*' allowed, bare quotes
  rejected), closing the unbalanced-quote gap in default-value validation
- Add tests: unbalancedQuoteInFunctionDefaultIsRejected,
  balancedQuotedArgsInFunctionDefaultAreAccepted
- Fix extra closing paren typo in the c3 case
…intainer review (OtterMind#1914)

- new XugudbIdentifierProcessor (SPI ISQLIdentifierProcessor): quoteIdentifier
  with double-quote doubling, escapeString with single-quote doubling
- XUGUDBMetaData overrides getSQLIdentifierProcessor(); metadata call sites use it
- builder/manager/enums use XugudbIdentifierProcessor.INSTANCE
- non-escapable validation moved to XugudbSqlGuards (column defaults, length units)
- XugudbSqlEscapes removed; tests migrated (21 green)
…rough, conditional for SPI, always for DDL) (OtterMind#1914)

Found in re-review: processor was always-quote with null->"" garbage,
breaking SPI completion paths; no quoteIdentifierAlways existed.

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer review complete. Rebased onto current main, fixed XuguDB identifier/default/type/builder semantics, and verified with the nine-module clean reactor suite (195/195 tests passed).

@openai0229
openai0229 merged commit 81f78f9 into OtterMind:main Jul 29, 2026
19 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Security: 34 SQL Injection Vulnerabilities in Database Plugins

3 participants